3.x: Make TabletMap return empty replica set when stale replica is found #385
Conversation
Bouncheck
force-pushed
the
scylla-3.x-remove-tablets-w-stale-hosts
branch
from
4caf8af to
7ee8323
Compare
There was a problem hiding this comment.
I wonder, why does this method return a set instead of some vector / list (in general: some ordered collection)? Order of the replicas matters for LWT.
Another note: this method is executed for each query, and requires allocations to be made to transform a list of hosts to lists of uuids. Why not do this transformation once, when receiving a tablet?
Good points. I'll try to address both of them. When first implementing I likely did not think about LWT at all. Thanks for quick review. |
|
However all past I think I was just matching the convention when creating the version for the tablets. |
|
@Bouncheck, feel free create a separate PR/issue on this problem or fix it in this PR. To me it would be better to address it in PR for #383, or in completely separate one. |
Bouncheck
force-pushed
the
scylla-3.x-remove-tablets-w-stale-hosts
branch
from
f7ea063 to
493b19d
Compare
I tinkered with this a bit and with current design translating when receiving the tablet won't save me from calling The only place it is used, the surrounding code gets the Host instances from metadata again to translate from UUID to Host, so I've added a commit changing the return type to save on that translation. |
|
The switch to collections preserving ordering probably should be done not just for |
Makes TabletMap return empty collection for some calls to `getReplicas()`. If the current replica list for a tablet turns out to contain a host that is not present in current Metadata then empty collection is returned in an effort to misroute the query. This query will cause the tablet information to be updated when the result with the up to date information is received. It is possible that the query will be randomly routed correctly, but this case is significantly less likely with each subsequent query, although this can depend on underlying load balancing policy. This covers the node removal/replacement case of scylladb#378.
Bouncheck
force-pushed
the
scylla-3.x-remove-tablets-w-stale-hosts
branch
from
493b19d to
f05ba86
Compare
Having this method return set of hosts instead saves one `Map#get()` query per host. In theory, previously it was also possible that between `TabletMap#getReplicas()` returning and `Metadata#getReplicas()` translating uuids to hosts, some hosts could become invalid, making the translation represent them as nulls.
Bouncheck
changed the title
Makes TabletMap return empty collection for some calls to
getReplicas(...). If the current replica list for a tablet turns out to contain a host that is not present in current Metadata then empty collection is returned in an effort to misroute the query. This query will cause the tablet information to be updated when the result with the up to date information is received.It is possible that the query will be randomly routed correctly, but this case is significantly less likely with each subsequent query, although this can depend on underlying load balancing policy.
This covers the node removal/replacement case of #378.